Fix #4840: Alias class prototype for methods in loose mode #5560
Conversation
|
@oliverdon, thanks for your PR! By analyzing the history of the files in this pull request, we identified @existentialism, @loganfsmyth and @hzoo to be potential reviewers. |
hzoo
added
the
PR: Polish 💅
| if (!this.aliasInserted) { | ||
| const classProto = t.memberExpression(this.classRef, t.identifier("prototype")); | ||
| const protoDeclaration = t.variableDeclaration("var", [ | ||
| t.variableDeclarator(t.identifier(this.methodAlias), classProto), |
There was a problem hiding this comment.
This needs to be a unique identifier, not the known I'd _proto.
There was a problem hiding this comment.
Thanks, that makes sense.
would replacing line 10:
this.methodAlias = "_proto";with
this.methodAlias = this.path.scope.generateUidIdentifier("proto").name;be a good way of achieving this?
There was a problem hiding this comment.
Made the change, and caught up changes from 7.0.
oliverdon
force-pushed
the
loose-class-reuse-prototype
branch
from
57f02e7
to
56fd0c6
Compare
| t.identifier("prototype"), | ||
| ); | ||
| const protoDeclaration = t.variableDeclaration("var", [ | ||
| t.variableDeclarator(t.identifier(this.methodAlias), classProto), |
There was a problem hiding this comment.
If you are going to use generateUidIdentifier, then you should just the identifier part and not do the .name 😄
this.methodAlias = this.path.scope.generateUidIdentifier("proto")
t.variableDeclarator(this.methodAlias, classProto),There was a problem hiding this comment.
Ah, good point 😅, done.
oliverdon
force-pushed
the
loose-class-reuse-prototype
branch
from
56fd0c6
to
53268dc
Compare
| @@ -6,6 +6,25 @@ export default class LooseClassTransformer extends VanillaTransformer { | |||
| constructor() { | |||
| super(...arguments); | |||
| this.isLoose = true; | |||
| this.aliasInserted = false; | |||
| this.methodAlias = this.path.scope.generateUidIdentifier("proto"); | |||
There was a problem hiding this comment.
Nit: this uses a global name. We can instead test if methodAlias is set yet, and create it when needed.
There was a problem hiding this comment.
Good point, removed this from constructor.
oliverdon
force-pushed
the
loose-class-reuse-prototype
branch
2 times, most recently
from
1897e78
to
ff11dce
Compare
jridgewell
force-pushed
the
loose-class-reuse-prototype
branch
from
ff11dce
to
c54ae28
Compare
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/4709/ |
|
@oliverdon thanks for your efforts! awesome stuff. ^ aside: btw we should figure out plugin options (loose mode) for the repl |
lock
bot
added
the
outdated
Alias class prototype for methods in loose mode.